-
Notifications
You must be signed in to change notification settings - Fork 154
chore(layers): add Idempotency, Batch, and Parameters to layer #1712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Integration tests for Node.js 14 & 16 are failing on the layer module (link).
Looking at the CloudWatch logs (see this LogGroup /aws/lambda/Layers-E2E-node14-11177-functionStack-testFn
) it appears the SDK is not being bundled on these versions.
Check here, I'm not sure which runtime the Lambda deployed for these tests is actually using.
I have added test groups for node14 and node16 and we have a similar problem we had before. AWS SDK clients are bundled by esbuild, the layer has all utilities, but the imports of AWS SDK can not be found. I am digging into esbuild and CDK bundling, and will also check SAM examples. |
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the latest fix the integration tests are passing: https://github.com/aws-powertools/powertools-lambda-typescript/actions/runs/6312038699
For the record and future readers, at this point the Lambda Layer includes all Powertools utilities and the AWS SDK clients for the services used by them (SSM, Secrets Manager, DynamoDB, and AppConfig). The total size is 10MB unzipped, we were able to shave off 8MB (-45%) by removing types, and ESM distributions of the SDKs that are shipped, and other files not needed at runtime that are shipped with some of the dependencies. There's a potential for additional optimization that would allow to decrease the Layer size by 35-50% (based on initial tests), however we have decided to not pursue it at this time mainly because of other competing priorities. The optimization would involve creating a barrel file (i.e. import {
AttributeValue,
DeleteItemCommand,
DynamoDBClient,
DynamoDBClientConfig,
DynamoDBServiceException,
GetItemCommand,
PutItemCommand,
UpdateItemCommand,
} from '@aws-sdk/client-dynamodb';
import { marshall, unmarshall } from '@aws-sdk/util-dynamodb';
export {
AttributeValue,
DeleteItemCommand,
DynamoDBClient,
DynamoDBClientConfig,
DynamoDBServiceException,
GetItemCommand,
marshall,
PutItemCommand,
unmarshall,
UpdateItemCommand,
}; the file would then be run through
this would generate an optimized (& potentially minified) version of the SDK that bundles only the code needed. We would then have to programmatically change the imports within the Powertools code to require this file rather than the SDK, i.e. const client_dynamodb_1 = require("@aws-sdk/client-dynamodb");
const util_dynamodb_1 = require("@aws-sdk/util-dynamodb"); would become const client_dynamodb_1 = require("../../../commons/lib/sdk");
const util_dynamodb_1 = require("../../../commons/lib/sdk"); We were able to test this and confirm that: 1/ a Layer created with this method works with all Node.js runtimes, 2/ the total size is significantly lower than the current one. |
Description of your changes
This PR adds idempotency utilities to the public layer. Thanks to the outstanding work in #1708 and #1710 this PR became lightweight.
I decided to add sdk clients to the tests, since it was a major concern as thoroughly discussed in #1708 .
Related issues, RFCs
Issue number: closes #1703
Checklist
Breaking change checklist
Is it a breaking change?: NO
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.